Error, rather than warn, once a number of invalid path operators are encountered in EvaluatorPreprocessor.read (bug 1443140)
#9838
Conversation
Snuffleupagus
force-pushed
the
invalid-path-OPS
branch
2 times, most recently
from
97bac55 to
56fcbb7
Compare
…encountered in `EvaluatorPreprocessor.read` (bug 1443140) Incomplete path operators, in particular, can result in fairly chaotic rendering artifacts, as can be observed on page four of the referenced PDF file. The initial (naive) solution that was attempted, was to simply throw a `FormatError` as soon as any invalid (i.e. too short) operator was found and rely on the existing `ignoreErrors` code-paths. However, doing so would have caused regressions in some files; see the existing `issue2391-1` test-case, which was promoted to an `eq` test to help prevent future bugs. Hence this patch, which adds special handling for invalid path operators since those may cause quite bad rendering artifacts. You could, in all fairness, argue that the patch is a handwavy solution and I wouldn't object. However, given that this only concerns *corrupt* PDF files, the way that PDF viewers (PDF.js included) try to gracefully deal with those could probably be described as a best-effort solution anyway. This patch also adjusts the existing `warn`/`info` messages to print the command name according to the PDF specification, rather than an internal PDF.js enumeration value. The former should be much more useful for debugging purposes. Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1443140.
Snuffleupagus
force-pushed
the
invalid-path-OPS
branch
from
56fcbb7 to
7f21e38
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/a0e954a9368ca7c/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/5f5c78b045a5b89/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/5f5c78b045a5b89/output.txt Total script time: 23.47 mins
|
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/a0e954a9368ca7c/output.txt Total script time: 38.61 mins
|
|
I think it's good to have a fix for this, but I can't really comment on the heuristic itself since I'm not really familiar with this part of the codebase. @brendandahl or @yurydelendik Could you perhaps take a look here to see if the heuristic is OK? |
|
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/1d0b8bdef0975f3/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://54.215.176.217:8877/b9d09e49cddfcd8/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/b9d09e49cddfcd8/output.txt Total script time: 21.36 mins
|
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/1d0b8bdef0975f3/output.txt Total script time: 35.74 mins
|
|
Thank you! |
Error, rather than warn, once a number of invalid path operators are encountered in `EvaluatorPreprocessor.read` (bug 1443140)
Incomplete path operators, in particular, can result in fairly chaotic rendering artifacts, as can be observed on page four of the referenced PDF file.
The initial (naive) solution that was attempted, was to simply throw a
FormatErroras soon as any invalid (i.e. too short) operator was found and rely on the existingignoreErrorscode-paths. However, doing so would have caused regressions in some files; see the existingissue2391-1test-case, which was promoted to aneqtest to help prevent future bugs.Hence this patch, which adds special handling for invalid path operators since those may cause quite bad rendering artifacts.
You could, in all fairness, argue that the patch is a handwavy solution and I wouldn't object. However, given that this only concerns corrupt PDF files, the way that PDF viewers (PDF.js included) try to gracefully deal with those could probably be described as a best-effort solution anyway.
This patch also adjusts the existing
warn/infomessages to print the command name according to the PDF specification, rather than an internal PDF.js enumeration value. The former should be much more useful for debugging purposes.Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1443140.